fix(resolve): Improve multi-MSRV workspaces #14569
Conversation
For workflows like `cargo info`, this call will live on regardless of what we do with the resolver.
We do this by resolving for a package version that is compatible with the most number of MSRVs within a workspace. If a version requirement is just right, every package will get the highest MSRV-compatible dependency. If its too high, packages will get MSRV-incompatible dependency versions. This will happen regardless of what we do due to the nature of `"fallback"`. If its too low, packages with higher MSRVs will get older-than-necessary dependency versions. This is similar to the "some with and without MSRV" workspaces. When locking dependencies, we do report to users when newer MSRV/SemVer compatible dependencies are available to help guide them to upgrading if desired. Fixes rust-lang#14414
rustbot
added
A-dependency-resolution
epage
force-pushed
the
msrv-max
branch
2 times, most recently
from
6fac2ab
to
7d8007c
Compare
There was a problem hiding this comment.
Implementation-wise looks good. I'd like to hear back from @VorpalBlade for their opinion on this new behavior.
There was a problem hiding this comment.
It seems like a step in the right direction, as I understand it, the majority wins, which would solve my issue.
(However, if you go ahead and open a pr in the middle of the night, and then merge it before morning there is little timely feedback I can give. Given timezones, work hours etc it is not reasonable to expect feedback in less than 24 hours.)
There was a problem hiding this comment.
Yes, that was little time for feedback before merging but they noted elsewhere that your feedback didn't have to be blocking for merge as this is unstable. Its still important for stabilization!
| @@ -59,7 +59,7 @@ impl VersionPreferences { | |||
| /// | |||
| /// Sort order: | |||
| /// 1. Preferred packages | |||
| /// 2. [`VersionPreferences::max_rust_version`] | |||
| /// 2. Most compatible [`VersionPreferences::rust_versions`] | |||
There was a problem hiding this comment.
The unstable doc might need an update according, for the algorithm of determining workspace MSRV. However, from a user perspective, it is a bit hard to understand. I am not sure if we want to document it.
It is also hard to determine from a human eye, if it is a large workspace and people are adding/removing members a lot that changes the workspace-wide MSRV.
There was a problem hiding this comment.
I've updated the text. The unstable docs offer us an opportunity to evaluate the wording. Even once its stabilized, we can iterate as we see how people are dealing with it.
|
@bors r+ |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
Update cargo 19 commits in eaee77dc1584be45949b75e4c4c9a841605e3a4b..80d82ca22abbee5fb7b51fa1abeb1ae34e99e88a 2024-09-19 21:10:23 +0000 to 2024-09-27 17:56:01 +0000 - Update cc to 1.1.22 (rust-lang/cargo#14607) - feat: lockfile path implies --locked on cargo install (rust-lang/cargo#14556) - feat(toml): Add `autolib` (rust-lang/cargo#14591) - fix: correct error count for `cargo check --message-format json` (rust-lang/cargo#14598) - test: relax panic output assertion (rust-lang/cargo#14602) - feat(timings): support dark color scheme in HTML output (rust-lang/cargo#14588) - feat: add CARGO_MANIFEST_PATH env variable (rust-lang/cargo#14404) - fix(config): Don't double-warn about `$CARGO_HOME/config` (rust-lang/cargo#14579) - fix(cargo-rustc): give trailing flags higher precedence on nightly (rust-lang/cargo#14587) - feat: make lockfile v4 the default (rust-lang/cargo#14595) - perf: Improve quality of completion performance traces (rust-lang/cargo#14592) - test: Remove completion tests (rust-lang/cargo#14590) - feat: Add support for completing `cargo update <TAB>` (rust-lang/cargo#14552) - test: Migrate remaining with_stdout/with_stderr calls (rust-lang/cargo#14577) - fix(resolve): Improve multi-MSRV workspaces (rust-lang/cargo#14569) - chore: Bump MSRV to 1.81 (rust-lang/cargo#14585) - Add a `--dry-run` flag to the `install` command (rust-lang/cargo#14280) - fix(resolve): Don't list transitive, incompatible dependencies as available (rust-lang/cargo#14568) - feat(complete): Upgrade clap_complete (rust-lang/cargo#14573)
What does this PR try to resolve?
We do this by resolving for a package version that is compatible
with the most number of MSRVs within a workspace.
If a version requirement is just right, every package will get the
highest MSRV-compatible dependency.
If its too high, packages will get MSRV-incompatible dependency
versions.
This will happen regardless of what we do due to the nature of
"fallback".If its too low, packages with higher MSRVs will get older-than-necessary
dependency versions.
This is similar to the "some with and without MSRV" workspaces.
When locking dependencies, we do report to users when newer MSRV/SemVer
compatible dependencies are available to help guide them to upgrading if
desired.
Fixes #14414
How should we test and review this PR?
Is this the right behavior?
Additional information